feat(sdk): guard the delete organization flow - #1881
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughOrganization deletion now checks server-reported blockers and token balances. The UI disables deletion while checks load or blockers exist. The dialog displays processed precondition errors and token transfer guidance. ChangesOrganization deletion validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to The organization-delete preflight can still enable the delete control before invoice status is available, allowing users to attempt deletion without complete blocker guidance. The server continues to enforce blocked deletions, so the impact is limited and the change is mergeable with explicit owner awareness or follow-up. 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
web/sdk/client/views/general/components/delete-organization-dialog.tsx (1)
48-48: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDefer the balance query until the dialog opens.
GeneralView mounts
DeleteOrganizationDialogeven whenopenis false. CallinguseTokens()starts the billing query for every permitted General view. A failed request can also show the “Unable to fetch balance” toast before the user opens the dialog.Add an
enabled: openoption touseTokens, or mount the dialog only while it is open.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6e51689d-663d-4f44-83ac-03667d03f265
📒 Files selected for processing (2)
web/sdk/client/views/general/components/delete-organization-dialog.tsxweb/sdk/client/views/general/general-view.tsx
Coverage Report for CI Build 32818746747Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Warning No base build found for commit Coverage: 49.131%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
rohilsurana
left a comment
There was a problem hiding this comment.
Review against main. The guard follows the existing payment-issue pattern well. A few correctness and UX notes; the token-transfer wording is the one I would fix before this ships.
f15c922 to
4bf5ef9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f32f0d8-36f5-4f90-a760-824945d49de1
📒 Files selected for processing (1)
web/sdk/client/views/general/components/delete-organization-dialog.tsx
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.
The delete button greys out while the org has open invoices with a non-zero amount, and its hover tooltip says to pay them from the billing page first — the server refuses the delete in that state anyway, this stops the user before the failed call. The delete dialog warns when tokens remain on the billing account: deleting forfeits them, and support can transfer the amount to the user's bank account. Confirming the dialog is the user's consent. The confirm button also disables while the request is in flight, so repeated clicks cannot fire the delete twice, and a failed_precondition response shows the server's reasons instead of a generic error.
The token balance is only fetched while the delete dialog is open, so the General page stops firing a balance request (and a failure toast on accounts without balance support) on every visit. The delete button now stays disabled while the unpaid-invoice answer is still loading instead of briefly allowing a delete the server would refuse, and it trusts the server-filtered result instead of re-checking only the state. The token warning stops promising a bank transfer for the whole balance — only a purchased amount is transferable, matching the notice email. The failed_precondition toast uses err.message like the other handlers, and the open-invoice filters live in one shared helper used by both the delete guard (one row, no sort — existence is all it needs) and the billing page's payment-issue banner.
4bf5ef9 to
d8b1b73
Compare
The delete button now asks the server directly whether a delete would go through (CheckOrganizationDelete, added in #1894) instead of inferring one blocker from the invoice list. The button greys out for every blocker the real delete would refuse with — a paid subscription to downgrade, unpaid invoices, a token debt — and the tooltip shows one short instruction per kind of blocker, with counts for invoices and a fallback to the server's message for kinds this version does not know. While the check loads the button stays disabled; a failed check fails open since the server refuses independently. @raystack/proton moves to the proton main commit that ships the RPC. The shared open-invoice filters stay for the billing page's payment-issue banner.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Tested this end to end on a local sandbox: client-demo (Vite, port 3000) built from this branch, talking to a Frontier server on main (which already serves All cases were retested at the branch head (
The debt message language matches what the user sees elsewhere: the Tokens page shows the negative number under "Available tokens" (verified showing -300) next to an "Add tokens" button, and adding tokens genuinely unblocks the delete right away (case 12). Not covered, because a healthy server cannot produce them: the fail-open path when the check RPC itself errors, and the unknown-blocker-kind fallback to the generic message. Both are small branches that were reviewed by reading the code. |
0880299 to
11b2163
Compare
isLoading is false when a reopened dialog refetches over cached data, so a stale balance could show while the confirm was already clickable. The guard now keys off isFetching, which covers the initial load and every refetch.
11b2163 to
6706a75
Compare
When a delete is refused, the toast now shows the same short instructions the tooltip uses, built from the blocker types in the error's PreconditionFailure detail (read from the Connect JSON debug payload). Blocker kinds this version does not know, unreadable details, and every other unexpected error show a generic message instead of the raw server text, which carried internal ids. The not-found toast says plainly that the organization no longer exists. The instruction mapping moved to one shared helper used by both the tooltip and the toast.
Telling the user to contact support made the settlement their job. The balance is settled by the operator, so the message now says the support team will reach out to them. The wording stays generic on purpose: this code runs for any deployment, so it names no product or company.
Showing the number of tokens pending settlement has tax implications, so the warning no longer names an amount. It says the organization still has unused tokens, that deleting forfeits them, and that the support team will reach out to settle any that were purchased. The balance is still fetched, but only to decide whether to show the warning at all.
8f742fd to
aa6c40e
Compare
aa6c40e to
04abe4d
Compare
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
04abe4d to
274f3f3
Compare
Nothing in the gap touches this package: the removed deprecated permission fields belong to PermissionRequestBody, which the client SDK never uses.
Part of #1837 — the client half of the delete pre-flight. The server stack (#1865–#1894) is merged; this PR now contains all the SDK changes in one place.
Guards in the client SDK's General settings view:
CheckOrganizationDeleteRPC (shipped in feat(deleter): serve CheckOrganizationDelete for delete eligibility #1894,@raystack/protonbumped to the proton main commit that carries it). The button greys out for every blocker the real delete would refuse with — a paid subscription to downgrade, unpaid invoices, a token debt — and the hover tooltip shows one short instruction per kind of blocker ("Downgrade the subscription to the standard plan", "Pay the 2 open invoices from the billing page", "Contact support to settle the token balance"), with a fallback to the server's own message for kinds this version does not know. While the check loads the button stays disabled; a failed check fails open, since the server refuses a blocked delete independently.failed_preconditionresponse still surfaces the server's reasons in the error toast for anyone who reaches the server anyway.client/utils/invoice-queries.ts).🤖 Generated with Claude Code